Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: expose currencyCode on Product object in IAP module #25058

Merged
merged 1 commit into from Aug 21, 2020

Conversation

svirs
Copy link
Contributor

@svirs svirs commented Aug 20, 2020

Description of Change

Add currencyCode to Product object that inAppPurchase.getProducts returns. resolves #25053

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • relevant documentation is changed or added
  • PR title follows semantic commit guidelines
  • PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.
  • This is NOT A BREAKING CHANGE. Breaking changes may not be merged to master until 11-x-y is branched.

Release Notes

Notes: Added the currencyCode field that Apple's StoreKit in-app-purchasing library provides but has not been added to the Product object that inAppPurchase.getProducts returns.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 20, 2020
@welcome
Copy link

welcome bot commented Aug 20, 2020

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@svirs
Copy link
Contributor Author

svirs commented Aug 20, 2020

would love to have this backported to older versions :)

@svirs svirs force-pushed the IAP_add_currencyCode_to_Product branch from 557a9ad to 94ca91a Compare August 20, 2020 07:48
@svirs
Copy link
Contributor Author

svirs commented Aug 20, 2020

First PR to electron, exciting! Here's some reasoning for this (which I'm not sure if it belongs in this comment or up top):

Currency codes are often used in the front and backend for various reasons. Having that information readily available let's us categorize the expected income better and create estimations of revenue much more easily since we can take the currency that's there and get an idea of the value of the transaction. It is also often used by 3rd party billing services for the magic blackbox that is their products. On the front end, while we currently are able to get properly formatted string for the price, we can't really use the info that is there to 'continue the conversation' so to speak, here's an example:

const products = inAppPurchase.getProducts(['mysku']);
if (products.length > 0) {
    const productPrice = products[0].price;
    const currency = products[0].currencyCode;
    const locale = 'de-DE';
    const currencyLocalizer = new Intl.NumberFormat(locale, { style: 'currency', currency })
    const tenLocalizedBucks = currencyLocalizer.format(10)
    alert(`You can get a subscription for {productPrice}! Isn't the savings incredible? Tip your barista {tenLocalizedBucks} the next time you get a latte!`);
}

If the currency code came back as 'EUR' we'd see "10,00 €", but if it came back as something else from Apple such as "XPF" we'd see "10 CFPF" instead.

Apple is the source of truth for the currency a user is shown, we can't rely on anything else if we want the way we show pricing to be consistent across not only the app but also the native checkout dialogs.

Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me

@svirs svirs force-pushed the IAP_add_currencyCode_to_Product branch 4 times, most recently from e48cd2b to 0d73d7c Compare August 20, 2020 10:06
@codebytere
Copy link
Member

@svirs looks like lint is failing:

$ node ./script/lint.js && npm run lint:clang-format && npm run lint:docs
shell/browser/mac/in_app_purchase_product.mm:153:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
Total errors found: 1

this lgtm otherwise tho!

@svirs svirs force-pushed the IAP_add_currencyCode_to_Product branch from 0d73d7c to b007d1c Compare August 20, 2020 16:13
@svirs
Copy link
Contributor Author

svirs commented Aug 20, 2020

@codebytere fixed!

@svirs
Copy link
Contributor Author

svirs commented Aug 20, 2020

Would it all right to label this with target/9-x-y?

@MarshallOfSound
Copy link
Member

@svirs That would require a minor bump and therefore needs to go through the release WG. I can tag it for review though 😄

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 21, 2020
@codebytere codebytere merged commit a17e97c into electron:master Aug 21, 2020
@welcome
Copy link

welcome bot commented Aug 21, 2020

Congrats on merging your first pull request! 🎉🎉🎉

@release-clerk
Copy link

release-clerk bot commented Aug 21, 2020

Release Notes Persisted

Added the currencyCode field that Apple's StoreKit in-app-purchasing library provides but has not been added to the Product object that inAppPurchase.getProducts returns.

@trop
Copy link
Contributor

trop bot commented Aug 21, 2020

I have automatically backported this PR to "10-x-y", please check out #25084

@codebytere
Copy link
Member

(tagging the actual backports with backport/requested instead)

@trop
Copy link
Contributor

trop bot commented Aug 21, 2020

I have automatically backported this PR to "9-x-y", please check out #25085

@svirs
Copy link
Contributor Author

svirs commented Aug 21, 2020

Woo! Looking forward to the nightly thanks y'all!

@svirs svirs deleted the IAP_add_currencyCode_to_Product branch August 21, 2020 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose currencyCode for in-app-purchase products
3 participants